Skip to content

Add support for Bazel - #2505

Merged
vitaut merged 1 commit into
fmtlib:masterfrom
Vertexwahn:add-bazel-support
Sep 24, 2021
Merged

vitaut merged 1 commit into
fmtlib:masterfrom
Vertexwahn:add-bazel-support

Conversation

@Vertexwahn

@Vertexwahn Vertexwahn commented Sep 17, 2021

Copy link
Copy Markdown
Contributor

Goal of this PR

This PR helps users of the Bazel build system to integrate fmt into their builds.

Supporting two build systems (CMake and Bazel) is of course more effort to maintain but helps to spread fmt.
In the worst case the Bazel build does simply not work - in the best case, someone who uses Bazel can benefit from it.

Also, other projects support CMake and Bazel side by side (e.g. Catch2, GoogleTest, OpenEXR, oneTBB etc.)

Demo

A Demo of how to use this can be found here.

Everything a user has to do is:

Create a WORKSPACE.bazel file with the following content:

load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")

# Fetch fmt
git_repository(
    name = "fmt",
    branch = "add-bazel-support",
    remote = "https://github.com/Vertexwahn/fmt",
)

Create a BUILD.bazel file and add a dependency to fmt:

cc_binary( # Build a binary
    name = "Demo", # Name of the Binary
    srcs = ["main.cpp"], # List of files - we only have main.cpp
    deps = ["@fmt//:fmt"], # Depend on fmt
)

Make use of fmt in main.cpp:

#include "fmt/core.h"

#include <iostream>

int main() {
    std::string message = fmt::format("The answer is {}.", 42);

    std::cout << message << std::endl;
    
    return 0;
}

Tested with: GCC 9.3.0, Clang12, MSVC 2019

Support

I am willing to support the Bazel build support of fmt for a half year - after this period I can reconsider doing it for another half year

@vitaut

vitaut commented Sep 18, 2021

Copy link
Copy Markdown
Contributor

Thanks for the PR. I don't think we should put these build files in the top-level directory but if they can be moved to the support directory where we already have gradle files, I'll be happy to merge.

@Vertexwahn

Vertexwahn commented Sep 18, 2021

Copy link
Copy Markdown
Contributor Author

Bazel requires that the WORKSPACE.bazel file is placed in the root folder. Moving it e.g. tosupport/bazel/WORKSPACE.bazel does not work, since Bazel can only access files in subfolders of the WORKSPACE file.

The BUILD file can also access only the subdirectories. Therefore, I see currently now way to move it away from the root folder.

The .bazelrc is kind of optional. It could simply be deleted. In its current form, it just avoids that some temporary Bazel-related folders are created.

The .bazelversion file can also be deleted. It is just a file for Bazelisk and a nice way to document which Bazel version was aimed for/tested with (I am pretty sure that it works with almost every Bazel version).

If you want to keep the number of files in the root folder minimal I can easily delete .bazelversion and .bazelrc. Nevertheless, WORKSPACE.bazel and BUILD.bazel need to stay.

A different approach would be to move the WORKSPACE.bazel and BUILD.bazel files to some folder (e.g. support/bazel/) and add a README.md to explain that those files have to be copied over to the root folder to get the whole thing working with Bazel.

Having the WORSPACE and BUILD files in the root folder is really the game-changer. This way the above-shown example works out of the box.

I see three options:

  1. Keep it as it is (best option in my opinion)
  2. Keep WORSPACE and BUILD files in the root folder - get rid of .bazelversion and .bazelrc (a good alternative in my opinion to the first approach)
  3. Move Bazel related files to support/bazel (better than nothing, but not very practical and does not allow easy integration as shown in the example)

What option do you prefer? 1, 2, 3?

@vitaut

vitaut commented Sep 19, 2021

Copy link
Copy Markdown
Contributor

I prefer going with option 3. One extra step to copy build files is not critical and keeping the project directory clean is important.

@Vertexwahn

Copy link
Copy Markdown
Contributor Author

Ok, I changed everything to option 3. I also found a way to use the current state out of the box with Bazel. Details about this can be found in the provided README.md file under section Alternative approach.

Comment thread support/bazel/README.md

@alexezeder alexezeder left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few IMHOs, feel free to ignore 😉

Comment thread support/bazel/BUILD.bazel
Comment thread support/bazel/README.md Outdated
Comment thread support/bazel/README.md Outdated
Comment thread support/bazel/README.md Outdated
Comment thread support/bazel/README.md Outdated
Comment thread support/bazel/README.md Outdated
Comment thread support/bazel/README.md Outdated
Comment thread support/bazel/WORKSPACE.bazel
@Vertexwahn

Vertexwahn commented Sep 22, 2021

Copy link
Copy Markdown
Contributor Author

Added changes as requested. Header-only version is currently not part of this. Can be one of the next steps.

Comment thread support/bazel/BUILD.bazel
Comment thread support/bazel/README.md Outdated
@vitaut
vitaut merged commit d9a731d into fmtlib:master Sep 24, 2021
@vitaut

vitaut commented Sep 24, 2021

Copy link
Copy Markdown
Contributor

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants